Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(providers): single database mode #1101

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vkrizan
Copy link
Contributor

@vkrizan vkrizan commented Dec 9, 2024

Creates a new database provider mode named single.
The aim of the mode is to create a single database instance and share it
between the apps by separating them by Posgres schemas.
Each app gets a schema created by the name of the requested database.
Schemas are created under environment's database, with the same name as
the ClowdEnv.

Adds the following refactorings:

  • provutils.DefaultPGPort to set port constant
  • provutils.DefaultPGAdminUsername to set default PG admin user
  • provutils.PGAdminConnectionStr that builds sanitized connection string to PG from a DatabaseConfig using admin creds
  • provutils.ReadDbConfigFromSecret to reuse read of DatabaseConfigs from secrets, optionally using a cache

RHINENG-14526

@vkrizan
Copy link
Contributor Author

vkrizan commented Dec 9, 2024

@psav I've opted for "single" as the name of the db mode. Suggestions and reviews are welcome. Thank you.

@vkrizan vkrizan force-pushed the single-db-provider branch 2 times, most recently from e0a2a56 to 86ee564 Compare December 20, 2024 14:44
@vkrizan vkrizan force-pushed the single-db-provider branch from 86ee564 to 5d02dad Compare January 13, 2025 14:18

var pgPassword string
if setAdmin {
pgPassword, err = utils.RandPassword(16, provutils.RCharSet)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought was to disable admin user for apps, however some apps might still require this to do some setups (i.e. create additional users). @psav any suggestions how we should approach this situation? I guess, one of the option is to update those services needing such privileges, or look at the privileges case-by-case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tricky because ephemeral usage often relies on them needing to do more stuff than usual, but in prod they will just get the credentials which are appropriate

@vkrizan
Copy link
Contributor Author

vkrizan commented Jan 13, 2025

I'm opening this up from a draft state for review. There still might be some small issues in certain conditions. I prefer to get this out sooner to do a bunch of testing on a test cluster. Thank you.

@vkrizan vkrizan marked this pull request as ready for review January 13, 2025 14:33
Creates a new database provider mode named `single`.
The aim of the mode is to create a single database instance and share it
between the apps by separating them by Posgres schemas.
Each app gets a schema created by the name of the requested database.
Schemas are created under environment's database, with the same name as
the ClowdEnv.

RHINENG-14526
@vkrizan vkrizan force-pushed the single-db-provider branch from 5d02dad to b7285df Compare January 13, 2025 14:55
@psav
Copy link
Collaborator

psav commented Jan 13, 2025

I'm opening this up from a draft state for review. There still might be some small issues in certain conditions. I prefer to get this out sooner to do a bunch of testing on a test cluster. Thank you.

We should talk about the expectation here and I was thinking we can deploy this for testing in the cluster - without merging it and only merge when we are ready - allows us to not goof up the cluster too much. @bsquizz @wcmitchell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants